-
Notifications
You must be signed in to change notification settings - Fork 354
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Redirect only GLib loggers to Journal #5962
base: master
Are you sure you want to change the base?
Conversation
Previously we redirected all output from the main Anaconda process to Journal to avoid GTK log messages (as GTK runs in the main process) from spamming TTY. Turns out this broke a couple things, such as the shell prompt in rescue mode. So drop the wholesale process output redirection and instead just redirect (hopefully) all GLib based loggers (used by GTK) to Journal. Related: RHEL-58834
Hello @M4rtinK! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:
|
So this seems to work well enough: A couple notes:
|
/kickstart-test --testtype smoke |
@@ -169,6 +169,7 @@ def setup_environment(): | |||
if "EDITOR" not in os.environ and os.path.isfile("/etc/profile.d/nano-default-editor.sh"): | |||
os.environ["EDITOR"] = "/usr/bin/nano" | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra newline
# Redirect all stderr messages from process to journal | ||
self.stderrToJournal() | ||
# Redirect GLib logging (eq. GTK) to Journal | ||
self.redirect_glib_logging_to_journal() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/eq./e.g.,/
looks like this file uses camelCase for function names? All of your new functions seem to use underscores
def stderrToJournal(self): | ||
"""Print all stderr messages from Anaconda to journal instead. | ||
def redirect_glib_logging_to_journal(self): | ||
"""Redirect GLib based library logging to Journal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's usually called "the journal" not "Journal", but I guess it doesn't really matter
|
||
Some GLib based libraries (such as GTK) do direct their | ||
sometimes quite verbose log messages to the output of the | ||
proces in which they are running. In the Anaconda case, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/proces/process/
sometimes quite verbose log messages to the output of the | ||
proces in which they are running. In the Anaconda case, | ||
this creates issues with TTY1 being spammed with those | ||
messages, with important content (such RDP connection intructions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/such RDP connection intructions/such as RDP connection instructions/
# create functions that convert the messages comming | ||
# from GLib into something that fits to the PYthon logging format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/comming/ s/PYthon/python/
It's not actually a python logging format but the anaconda logging format right?
self.anaconda_logger.debug("GLib: %s", message) | ||
return GLib.LogWriterOutput.HANDLED | ||
|
||
# redirect GLib loug output vit the functions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/loug/log/ s/vit/via/
return GLib.LogWriterOutput.HANDLED | ||
|
||
# redirect GLib loug output vit the functions | ||
GLib.log_set_handler(None, GLib.LogLevelFlags.LEVEL_MASK, log_adapter, None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i wonder if it would be clearer to give the individual log levels instead of using the mask wholesale. doesn't really matter, but food for thought.
Previously we redirected all output from the main Anaconda process to Journal to avoid GTK log messages (as GTK runs in the main process) from spamming TTY. Turns out this broke a couple things, such as the shell prompt in rescue mode.
So drop the wholesale process output redirection and instead just redirect (hopefully) all GLib based loggers (used by GTK) to Journal.
Related: RHEL-58834